Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(baseuri): add baseuri-range config property to control servers the Agent will register with #243

Merged
merged 1 commit into from
Nov 6, 2023
Merged

feat(baseuri): add baseuri-range config property to control servers the Agent will register with #243

merged 1 commit into from
Nov 6, 2023

Conversation

andrewazores
Copy link
Member

Fixes #242

ex. try patching the usual Cryostat smoketest script as follows:

diff --git a/smoketest.sh b/smoketest.sh
index 187c5a7e..409808e9 100755
--- a/smoketest.sh
+++ b/smoketest.sh
@@ -211,7 +211,8 @@ runDemoApps() {
         --env CRYOSTAT_AGENT_WEBSERVER_HOST="localhost" \
         --env CRYOSTAT_AGENT_WEBSERVER_PORT="9977" \
         --env CRYOSTAT_AGENT_CALLBACK="http://localhost:9977/" \
-        --env CRYOSTAT_AGENT_BASEURI="${protocol}://localhost:${webPort}/" \
+        --env CRYOSTAT_AGENT_BASEURI_RANGE="loopback" \
+        --env CRYOSTAT_AGENT_BASEURI="${protocol}://cryostat:${webPort}/" \
         --env CRYOSTAT_AGENT_TRUST_ALL="true" \
         --env CRYOSTAT_AGENT_AUTHORIZATION="Basic $(echo user:pass | base64)" \
         --env CRYOSTAT_AGENT_HARVESTER_PERIOD_MS=60000 \

then do podman logs -f quarkus-test-agent-1. This should produce log output like:

podman logs -f quarkus-test-agent-1 
Starting the Java application using /opt/jboss/container/java/run/run-java.sh ...
INFO exec  java -Dquarkus.http.host=0.0.0.0 -Djava.util.logging.manager=org.jboss.logmanager.LogManager -Dcom.sun.management.jmxremote.port=9097 -Dcom.sun.management.jmxremote.ssl=false -Dcom.sun.management.jmxremote.authenticate=false -javaagent:/deployments/app/cryostat-agent.jar -XX:+UseParallelGC -XX:MinHeapFreeRatio=10 -XX:MaxHeapFreeRatio=20 -XX:GCTimeRatio=4 -XX:AdaptiveSizePolicyWeight=90 -XX:+ExitOnOutOfMemoryError -cp "." -jar /deployments/quarkus-run.jar 
SLF4J: Class path contains multiple SLF4J providers.
SLF4J: Found provider [io.cryostat.agent.shaded.org.slf4j.simple.SimpleServiceProvider@26a1ab54]
SLF4J: Found provider [io.cryostat.agent.shaded.org.slf4j.jul.JULServiceProvider@3d646c37]
SLF4J: See https://www.slf4j.org/codes.html#multiple_bindings for an explanation.
SLF4J: Actual provider is of type [io.cryostat.agent.shaded.org.slf4j.simple.SimpleServiceProvider@26a1ab54]
2023-11-06 17:28:51:662 +0000 [cryostat-agent-main] INFO io.cryostat.agent.Agent - Cryostat Agent starting...
2023-11-06 17:28:51:713 +0000 [cryostat-agent-main] ERROR io.cryostat.agent.Agent - Agent startup failure
java.lang.IllegalArgumentException: cryostat.agent.baseuri of "https://cryostat:8181/" is unacceptable with URI range "LOOPBACK"
	at io.cryostat.agent.Agent.main(Agent.java:56)
	at io.cryostat.agent.Agent.lambda$agentmain$2(Agent.java:143)
	at java.base/java.lang.Thread.run(Thread.java:833)
__  ____  __  _____   ___  __ ____  ______ 
 --/ __ \/ / / / _ | / _ \/ //_/ / / / __/ 
 -/ /_/ / /_/ / __ |/ , _/ ,< / /_/ /\ \   
--\___\_\____/_/ |_/_/|_/_/|_|\____/___/   
2023-11-06 17:28:52,195 INFO  [io.quarkus] (main) quarkus-test 1.0.0-SNAPSHOT on JVM (powered by Quarkus 2.7.2.Final) started in 0.456s. Listening on: http://0.0.0.0:10010
2023-11-06 17:28:52,195 INFO  [io.quarkus] (main) Profile prod activated. 
2023-11-06 17:28:52,196 INFO  [io.quarkus] (main) Installed features: [cdi, rest-client, rest-client-jackson, resteasy, smallrye-context-propagation, vertx]
2023-11-06 17:29:01,889 INFO  [io.quarkus] (Shutdown thread) quarkus-test stopped in 0.015s

indicating that the Agent instance has refused to start because the server base URI provided uses the hostname cryostat, which the Agent cannot guarantee resolves to an address in the specified LOOPBACK range.

If the smoketest script is not patched to override the baseuri-range then the Agent instances fall back to the default DNS_LOCAL range from this patch, which will accept the smoketest's normal localhost value, as well as values like $HOSTNAME.local or dummy.localhost.

@github-actions github-actions bot added the needs-triage Needs thorough attention from code reviewers label Nov 6, 2023
@mergify mergify bot added the safe-to-test label Nov 6, 2023
@andrewazores andrewazores removed the needs-triage Needs thorough attention from code reviewers label Nov 6, 2023
@andrewazores andrewazores added the feat New feature or request label Nov 6, 2023
ebaron
ebaron approved these changes Nov 6, 2023
View reviewed changes
Copy link
Member

@ebaron ebaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@ebaron ebaron added the backport label Nov 6, 2023
@andrewazores andrewazores merged commit e02ba92 into cryostatio:main Nov 6, 2023
19 of 20 checks passed
@andrewazores andrewazores deleted the server-uri-host-check branch November 6, 2023 20:47
mergify bot pushed a commit that referenced this pull request Nov 6, 2023
@andrewazores @mergify
feat(baseuri): add baseuri-range config property to control servers t…
4d4b5e3 
…he Agent will register with (#243)

(cherry picked from commit e02ba92)
@mergify mergify bot mentioned this pull request Nov 6, 2023
Merged
andrewazores added a commit that referenced this pull request Nov 6, 2023
@mergify @andrewazores
feat(baseuri): add baseuri-range config property to control servers t…
9cd618f 
…he Agent will register with (#243) (#244)

(cherry picked from commit e02ba92)

Co-authored-by: Andrew Azores <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebaron ebaron ebaron approved these changes

Assignees
No one assigned
Labels
backport feat New feature or request safe-to-test
Projects
No open projects
2.4.0 release
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Task] Require server URL to use private-range IP by default
2 participants
@andrewazores @ebaron